-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
vmsdk: add vsock support on TDX quote fetching #4
Conversation
src/python/cctrusted_vm/cvm.py
Outdated
|
||
tdvmcall_flag = False | ||
# Use vsock to get TD Quote by default | ||
with open(TdxVM.CFG_FILE_PATH, 'rb') as port_file: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to check whether CFG_FILE_PATH exists and raise exception if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
# Fetch quote through tdvmcall | ||
# pylint: disable=E1111 | ||
req_buf = quote_req.prepare_reqbuf(report_bytes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it miss an "else" here corresponding to line 414? Otherwise it will execute line 452 fetching quote through tdvmcall anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the end of the 'if' condition, it just returns the tdquote as output. Will skip the tdvmcall part.
src/python/cctrusted_vm/cvm.py
Outdated
tdvmcall_flag = False | ||
# Use vsock to get TD Quote by default | ||
with open(TdxVM.CFG_FILE_PATH, 'rb') as port_file: | ||
port_info = port_file.read().rstrip().decode("utf-8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What will happen when there is no config file?
- Maybe port_file -> config_file is better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/python/cctrusted_vm/cvm.py
Outdated
# Use vsock to get TD Quote by default | ||
with open(TdxVM.CFG_FILE_PATH, 'rb') as port_file: | ||
port_info = port_file.read().rstrip().decode("utf-8") | ||
if "port=" not in port_info: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it will be false when I configure it to "port =xx" or "port=xx", right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Changed by trimming all spaces and do the comparsion.
src/python/cctrusted_vm/cvm.py
Outdated
msg_size_bytes = [ | ||
(msg_size >> 24) & 0xFF, | ||
(msg_size >> 16) & 0xFF, | ||
(msg_size >> 8) & 0xFF, | ||
msg_size & 0xFF | ||
] | ||
|
||
p_blob_payload = bytearray(header_size) | ||
for i, byte_value in enumerate(msg_size_bytes): | ||
p_blob_payload[i] = byte_value | ||
p_blob_payload[header_size:header_size + msg_size] = qgs_msg[:msg_size] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe struct.pack
is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for pointing out. Found another .to_bytes() func to fulfill the need.
Signed-off-by: Ruoyu Ying <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.